-
Notifications
You must be signed in to change notification settings - Fork 94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Record workflow version control information post-install #4142
Conversation
This comment has been minimized.
This comment has been minimized.
670f005
to
4488a2e
Compare
cbd0350
to
7979ff8
Compare
""" | ||
if not info: | ||
raise ValueError("Nothing to write") | ||
info_file = Path(run_dir, 'log', 'version', 'vcs.conf') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't see a way to get the timestamp of the installation, perhaps this needs to be added to the entry point arguments?
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments. I have tested the change as working, read the code and run the tests. Thanks Ronnie.
return None | ||
|
||
|
||
def _run_cmd(vcs: str, args: Iterable[str], cwd: Union[Path, str]) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth making use of run_cmd
in remote.py
? Could adapt it if you need to pass the cwd?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, but it wouldn't quite work here due to the difference in error handling
Just remembered a question I had: do we want to save the diff of untracked files for a git source dir? This is actually not the default behaviour of git, |
Could those untracked files make a difference to the running of the workflow? For example, if they had configured their .cylcignore file to ignore directories for the installation but this was not under source control, they may wish to know about it. Although perhaps this could lead to the log filling with unwanted diffs... difficult choice. I guess by marking files as untracked they should assume it would not be included in the diff, perhaps a one line warning at the top reminding them that there may be untracked changed? |
Yes
I don't think there is a way of marking files as untracked. It's pretty much only newly-created files that haven't been The untracked files will still show up in the
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, just need to give it a spin.
cylc/flow/post_install/__init__.py
Outdated
Built In Plugins | ||
---------------- | ||
|
||
Cylc Flow provides the following post-install plugins: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can document the pre-install and post-install entry points together as "install plugins" since they have related functionality and plugins may use both (e.g. cylc-rose
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with Git, all good.
- Needs a quick changelog entry.
- Can you squash down commits, namely "Tidy" ones.
|
||
"""Record version control information on workflow install. | ||
|
||
The supported version control systems are git and svn. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From cylc/cylc-doc#236:
Could do with a little more documentation here, a quick outline of:
- Where the version control information is stored.
- The format of this file.
- The meaning of each field.
Can be quite short.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
repository version = "2.8.0-dirty" | ||
commit = "e5dc6573dd70cabd8f973d1535c17c29c026d553" | ||
working copy root path = "~/cylc-src/my-workflow-git" | ||
status = \"\"\" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think these quotes should be escaped here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes close #3849
On installation of a workflow, will record the version control status to
log/version/vcs.conf
and any uncommitted diff tolog/version/uncommitted.diff
.Supports git and svn.
Requirements check-list
CONTRIBUTING.md
and added my name as a Code Contributor.